Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework CLI and worker spawning to enhance the distributed worker feature #742

Merged
merged 1 commit into from
Feb 27, 2020

Conversation

aklenik
Copy link
Contributor

@aklenik aklenik commented Feb 26, 2020

Signed-off-by: Attila Klenik [email protected]

Moderate-sized PR to enhance the distributed worker mode and CLI UX.

Adapter changes

  • The Worker.js and WorkerFactory.js files have been removed for every adapter.
  • An adapterFactory.js module has been added to every adapter, that exports a factory function for the adapter.
  • This factory function is the only exported object of the adapter packages
  • The adapters only receive the worker index for the factory. Retrieval of the configuration files is done inside the adapter (could be moved outside a layer for easier testing).

Fabric adapter fix

  • Fixed the workerInit flag usage in the init adapter methods
  • Bumped SDK versions to 1.4.7

Docker build improvements

  • Different tag possibility for docker image, not just the unique version (convenient for local testing)
  • Caliper is now installed globally in the image, using a nice entry point/cmd pattern (defaulting to "caliper --version", could be changed)

General updates

  • A common mechanism for loading arbitrary factory functions from any module (the basis of the envisioned plugin mechanism)
  • The process/worker life-cycle has been improved a bit to prevent hanging workers after rounds (or early exits due to CLI termination)
  • The MQTT connection is now cleaned up (messenger.dispose) at the end of the benchmark (a unified resource life-cycle management would be good for every object type, like adapters, monitors, etc)
  • The network config path is no longer saved in the BlockchainInterface base class (pointless restriction)
  • Error stacks are also printed along with the error message (we need to unify this along with the other logging todo)

CLI updates

  • Binding SUT and SDK params are merged (e.g., fabric:1.4.7)
  • Common "launch" command for starting "master" and "worker" processes
  • If the binding specification is set, then the launch commands perform the binding automatically (except for spawned worker processes)
  • The "launch worker" command now contains the previous Worker.js content in a unified way.
  • Adapters are now instantiated through the new factory functions (and the previous worker factories have been removed from the different orchestrators)

Worker management updates

  • Workers are now spawned through the unified CLI command
  • Explicit exit message upon finishing all rounds

CI tests

  • Updated the CLI commands
  • Created fully Docker-based non-CI tests for easily checking local changes in complex scenarios:
    • A Fabric test, using locally spawned workers.
    • A Fabric test, using distributed workers.

Copy link
Contributor

@nklincoln nklincoln left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only able to spot an issue in the naming of the launch worker command ... it refers to the launch master process instead 🤷‍♂

Loving the changes 👍

packages/caliper-cli/lib/launch/lib/launchWorker.js Outdated Show resolved Hide resolved
packages/caliper-cli/lib/launch/lib/launchWorker.js Outdated Show resolved Hide resolved
Copy link
Contributor

@nklincoln nklincoln left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😍

@aklenik aklenik merged commit fc3e458 into hyperledger-caliper:master Feb 27, 2020
@aklenik aklenik deleted the cli-enhancement branch February 27, 2020 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants